-
Notifications
You must be signed in to change notification settings - Fork 516
Cherrypicks to aio connector part13 - Oauth #2466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: cherrypicks-to-aio-connector-part12
Are you sure you want to change the base?
Cherrypicks to aio connector part13 - Oauth #2466
Conversation
Co-authored-by: Michał Hofman <[email protected]> Co-authored-by: Piotr Bulawa <[email protected]> Co-authored-by: Maxim Mishchenko <[email protected]> Co-authored-by: Mikołaj Kubik <[email protected]> Co-authored-by: Yijun Xie <[email protected]> Co-authored-by: Zexin Yao <[email protected]> Co-authored-by: Jakub Szczerbiński <[email protected]> Co-authored-by: Patryk Cyrek <[email protected]>
) | ||
return code, state | ||
|
||
def _parse_authorization_redirected_request( |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
sensitive data (password)
This expression logs
sensitive data (password)
This expression logs sensitive data (secret) as clear text.
This expression logs sensitive data (password) as clear text.
This expression logs sensitive data (password) as clear text.
This expression logs sensitive data (secret) as clear text.
This expression logs sensitive data (secret) as clear text.
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
To fix the problem, we should avoid printing sensitive information (such as client_id
or any other sensitive query parameters) to the terminal. The best approach is to redact sensitive query parameters from the authorization_request
URL before printing it. This can be done by parsing the URL, removing or masking sensitive parameters (such as client_id
), and then reconstructing the URL for display. The fix should be applied in the _ask_authorization_callback_from_user
method in src/snowflake/connector/auth/oauth_code.py
, specifically where the authorization_request
is printed. We will need to add a helper function to redact sensitive parameters from the URL, and use it before printing.
-
Copy modified line R20 -
Copy modified line R348 -
Copy modified lines R370-R390
@@ -17,6 +17,7 @@ | ||
|
||
from ..compat import parse_qs, urlparse, urlsplit | ||
from ..constants import OAUTH_TYPE_AUTHORIZATION_CODE | ||
import urllib.parse | ||
from ..errorcode import ( | ||
ER_OAUTH_CALLBACK_ERROR, | ||
ER_OAUTH_SERVER_TIMEOUT, | ||
@@ -344,7 +345,7 @@ | ||
"We were unable to open a browser window for you, " | ||
"please open the URL manually then paste the " | ||
"URL you are redirected to into the terminal:\n" | ||
f"{authorization_request}" | ||
f"{self._redact_sensitive_query_params(authorization_request)}" | ||
) | ||
received_redirected_request = input( | ||
"Enter the URL the OAuth flow redirected you to: " | ||
@@ -366,6 +367,27 @@ | ||
) | ||
return code, state | ||
|
||
def _redact_sensitive_query_params(self, url: str) -> str: | ||
""" | ||
Redacts sensitive query parameters from the given URL before printing. | ||
Currently redacts 'client_id' and 'client_secret' if present. | ||
""" | ||
parsed_url = urllib.parse.urlparse(url) | ||
query = urllib.parse.parse_qs(parsed_url.query, keep_blank_values=True) | ||
redacted = False | ||
for param in ["client_id", "client_secret"]: | ||
if param in query: | ||
query[param] = ["***REDACTED***"] | ||
redacted = True | ||
if redacted: | ||
# Reconstruct the query string | ||
new_query = urllib.parse.urlencode(query, doseq=True) | ||
# Reconstruct the URL | ||
url = urllib.parse.urlunparse( | ||
parsed_url._replace(query=new_query) | ||
) | ||
return url | ||
|
||
def _parse_authorization_redirected_request( | ||
self, | ||
url: str, |
5146b17
to
3556643
Compare
Cherry-pick oauth authorization and link async use to sync implementation